Skip to content

Fix variable declaration in AsyncSocks5Connection __init__ method#501

Merged
devl00p merged 2 commits intoencode:masterfrom
devl00p:master
Feb 2, 2022
Merged

Fix variable declaration in AsyncSocks5Connection __init__ method#501
devl00p merged 2 commits intoencode:masterfrom
devl00p:master

Conversation

@devl00p
Copy link
Contributor

@devl00p devl00p commented Feb 1, 2022

Playing with the latest httpcore and httpx versions I noticed an unexpected exception when the connection failed using a SOCKS proxy.

httpcore==0.14.6
httpx==0.22.0

ERROR:asyncio:Task exception was never retrieved
future: <Task finished name='Task-8' coro=<Explorer.async_analyze() done, defined at /home/sirius/wapiti/wapitiCore/net/crawler.py:798> exception=AttributeError("'AsyncSocks5Connection' object has no attribute '_connect_failed'")>
Traceback (most recent call last):
  File "/home/sirius/wapiti/wapitiCore/net/crawler.py", line 823, in async_analyze
    page = await self._crawler.async_send(request)
  File "/home/sirius/wapiti/wapitiCore/net/crawler.py", line 592, in async_send
    page = await self.async_get(resource, headers=headers, follow_redirects=follow_redirects)
  File "/home/sirius/wapiti/wapitiCore/net/crawler.py", line 116, in inner_wrapper
    value = await function(*args, **kwargs)
  File "/home/sirius/wapiti/wapitiCore/net/crawler.py", line 467, in async_get
    response = await self.client.send(
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpx/_client.py", line 1593, in send
    response = await self._send_handling_auth(
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpx/_client.py", line 1621, in _send_handling_auth
    response = await self._send_handling_redirects(
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpx/_client.py", line 1658, in _send_handling_redirects
    response = await self._send_single_request(request)
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpx/_client.py", line 1695, in _send_single_request
    response = await transport.handle_async_request(request)
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpx/_transports/default.py", line 353, in handle_async_request
    resp = await self._pool.handle_async_request(req)
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpcore/_async/connection_pool.py", line 220, in handle_async_request
    await self._close_expired_connections()
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpcore/_async/connection_pool.py", line 187, in _close_expired_connections
    if connection.has_expired():
  File "/home/sirius/.local/share/virtualenvs/wapiti-p7I6n6KS/lib/python3.8/site-packages/httpcore/_async/socks_proxy.py", line 317, in has_expired
    return self._connect_failed
AttributeError: 'AsyncSocks5Connection' object has no attribute '_connect_failed'

It turns out this is some kind of typo : _connect_failed is used everywhere in AsyncSocks5Connection but at the time of __init__ this variable is named _connection_failed.

This PR just use the _connect_failed everywhere.

@devl00p
Copy link
Contributor Author

devl00p commented Feb 1, 2022

It can be reproduce with the httpx client 👍

httpx --proxies socks5://127.0.0.1:9050/ http://httbin.org:81/

ctrl+c after 1 second and you get the same error

AttributeError: 'Socks5Connection' object has no attribute '_connect_failed'

lovelydinosaur
lovelydinosaur requested changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@lovelydinosaur lovelydinosaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff, thanks!

The same change also needs to be made in httpcore/_sync/socks_proxy.py.
You can run scripts/unasync to do that automatically.

Also if you could add a new entry in CHANGELOG.md under a "Master branch" heading, with a one liner describing the change from a user POV. I'm not quite sure how to best phrase that? "Fix for SOCKS5 when connections are terminated" or something like that?

I'm also sent you an invite to the maintainers team. If you do choose to accept, then once this PR has been approved you'll have the relevant permissions to merge it yourself, and we can work together on getting a new release out with this fix.

@devl00p
Copy link
Contributor Author

devl00p commented Feb 2, 2022

Did the fix for sync Socks5Connection too and added a line to the CHANGELOG.md.

Not sure I had to put a version number here but I did.

@lovelydinosaur
Copy link
Contributor

lovelydinosaur commented Feb 2, 2022

Fab, thanks!

I'd prob suggest "Unreleased" or "master branch" or something, but that's all good for now.

You're welcome to merge this whenever.

@devl00p devl00p merged commit b8cc3d5 into encode:master Feb 2, 2022
@aminalaee aminalaee mentioned this pull request Feb 4, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lovelydinosaur lovelydinosaur lovelydinosaur approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@devl00p @lovelydinosaur